test: Phase 1 baseline & containment for #610 - #614
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe PR adds Phase 1 regression tests, standalone lexer/parser/pattern/frontend fuzz targets, stable CI checks, workspace-wide test execution, fuzz lockfile synchronization, supported-platform and security documentation, fuzz seeds, and a deprecated time-format parser replacement. ChangesPhase 1 containment
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related issues
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
This PR completes Phase 1 (“baseline & containment”) items for the production-readiness tracker (#610) by adding an auditable correctness regression index, introducing a standalone fuzzing workspace, and documenting supported platforms/support boundaries, plus small supporting doc + build hygiene updates.
Changes:
- Add
tests/phase1_correctness_regression_test.rsas the single indexed regression suite for inventoried correctness defects (guards for fixed issues;#[ignore]reproducers for open ones). - Introduce an excluded, standalone
fuzz/cargo-fuzz workspace with four fuzz targets and tracked seed corpora. - Add supported-platform tiering documentation and link it from SECURITY/docs; fix a deprecated
timeformatting API usage in logging.
Reviewed changes
Copilot reviewed 25 out of 25 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| tests/phase1_correctness_regression_test.rs | New Phase 1 correctness regression index: fixed-defect guards + ignored reproducers for open defects. |
| src/logging.rs | Replace deprecated time::format_description::parse usage with parse_borrowed::<2>. |
| SECURITY.md | Update supported-version table to 26.7.x and link to supported-platform tiers doc. |
| Cargo.toml | Exclude fuzz/ from the root workspace build/test surface. |
| Docs/reference/supported-platforms.md | New reference defining platform tiers, toolchain/runtime requirements, and support boundaries. |
| Docs/README.md | Link the new supported-platforms reference from the docs index. |
| fuzz/README.md | Document fuzz targets, rationale for separate workspace, and how to run with cargo-fuzz. |
| fuzz/Cargo.toml | Define standalone cargo-fuzz workspace + four fuzz target bins. |
| fuzz/.gitignore | Ignore corpus/artifacts/coverage/lockfile for fuzz workspace hygiene. |
| fuzz/fuzz_targets/fuzz_pattern.rs | Fuzz pattern parse → compile → VM find_all pipeline for ReDoS-relevant surface. |
| fuzz/fuzz_targets/fuzz_parser.rs | Fuzz lex + Parser::parse for panic/overflow/hang resistance. |
| fuzz/fuzz_targets/fuzz_module_loading.rs | Fuzz module-content parsing + include/load detection + Analyzer::analyze. |
| fuzz/fuzz_targets/fuzz_lexer.rs | Fuzz lex_wfl_with_positions_checked for lexer robustness. |
| fuzz/seeds/fuzz_pattern/seed_word.txt | Seed input for pattern fuzzing (word-style quantifier form). |
| fuzz/seeds/fuzz_pattern/seed_mixed.txt | Seed input for pattern fuzzing (mixed constructs). |
| fuzz/seeds/fuzz_pattern/seed_digits.txt | Seed input for pattern fuzzing (simple digit chaining). |
| fuzz/seeds/fuzz_parser/seed_count.wfl | Seed program for parser fuzzing (count loop). |
| fuzz/seeds/fuzz_parser/seed_action.wfl | Seed program for parser fuzzing (action definition/call). |
| fuzz/seeds/fuzz_module_loading/seed_loadmod.wfl | Seed program for module-loading fuzzing (load module). |
| fuzz/seeds/fuzz_module_loading/seed_include.wfl | Seed program for module-loading fuzzing (include from). |
| fuzz/seeds/fuzz_lexer/seed_store.wfl | Seed program for lexer fuzzing (simple store/display). |
| fuzz/seeds/fuzz_lexer/seed_check.wfl | Seed program for lexer fuzzing (conditional syntax). |
| Dev diary/2026-07-13-issue-610-phase-1-scorecard-baseline.md | Evidence artifact for initial readiness scorecard baseline. |
| Dev diary/2026-07-13-issue-610-phase-1-inventory.md | Evidence artifact for issue inventory and severity classification. |
| Dev diary/2026-07-13-issue-610-phase-1-containment.md | Evidence artifact for Phase 1 containment tasks (tests/fuzz/platforms/metrics). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6b5bcca314
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let (out, code) = run_src( | ||
| "store a as 2 plus 3 times 4\n\ | ||
| display \"A=\" with a\n\ | ||
| store b as 10 divided by 4\n\ |
There was a problem hiding this comment.
Cover slash division in the #571 regression
This regression is documented as guarding #571's / lexing, but the program exercises the word form divided by instead. A future regression that breaks 10 / 4 while leaving 10 divided by 4 intact would still pass this Phase 1 suite, so the advertised correctness defect is not actually guarded here; use the slash form or add a separate assertion for it.
Useful? React with 👍 / 👎.
logbie
left a comment
There was a problem hiding this comment.
Deep review — request changes
I reviewed all 25 changed files at 6b5bcca314, the linked defect inventory, fuzz surfaces, support/security claims, current mergeability, and head-SHA CI state.
The direction is useful, but this is not merge-ready. The central claim—that Phase 1 containment is complete and evidence-backed—is stronger than the implementation and current CI evidence support.
Blocking findings
-
The branch is unmergeable and has no Actions validation.
The branch is behind
main, and merged PR #613 added the same inventory and scorecard files. Rebasing produces add/add conflicts in:Dev diary/2026-07-13-issue-610-phase-1-inventory.mdDev diary/2026-07-13-issue-610-phase-1-scorecard-baseline.md
GitHub currently reports
mergeable: false. There are zero GitHub Actions runs for this head; the only commit status is CodeRabbit success, while CodeRabbit's own comment says its review limit was reached.Fix: rebase onto current
main, reconcile rather than overwrite #613's evidence, and require a complete fresh CI run. -
The “every known correctness defect” gate is materially incomplete.
tests/phase1_correctness_regression_test.rs:189-192maps #578 to five selected tests, but #578 contains 26 unchecked items. Even its confirmed functional-bug section includes two missing cases: the nested-for eachcrash and incorrect friendly date/time formatting. This also contradictscontainment.md:124, which records “Known crashes / hangs: none.”Fix: classify every #578 checkbox independently, add desired-behavior regressions for each open correctness bug—at minimum the crash and date/time defects—and leave the Phase 1 gate open until the inventory is complete.
-
The #592 regression covers only half the reported defect.
tests/phase1_correctness_regression_test.rs:167-186tests only the top-level form. #592 also reports the failure inside an action body, so a partial fix could make this test green while the issue remains broken.Fix: parameterize the regression across top-level and action-body contexts and assert exact output plus exit code zero in both.
-
fuzz_module_loadingnever exercises module loading.fuzz/fuzz_targets/fuzz_module_loading.rs:15-36explicitly excludes filesystem/async resolution and only lexes/parses one in-memory string, calls two AST predicates, and invokes a fresh analyzer. It misses path resolution/canonicalization, bounded reads, circular/import-depth handling, checked lexing, parent scopes, type checking, and execution.Fix: drive the real loader using importer/module files in a temporary directory under Tokio. Otherwise rename this to static include-analysis fuzzing and leave the module-loading gate incomplete.
-
The baseline is presented as measured evidence, but it is extrapolated.
containment.md:106-119says1477/0/16was CI-measured on exact commitfc21f2f, but that commit is a[skip ci]version bump with no workflow run. The reported1479/0/22is arithmetic from the new suite's delta, not an observed full-suite result. The rationale that a local run requires live database services is also inaccurate; those tests skip when their environment variables are absent.Fix: after rebasing, run the complete suite on the head SHA and link the workflow/log, or clearly label the derived numbers as estimates.
-
The Tier-1 platform guarantees exceed actual CI coverage.
Docs/reference/supported-platforms.md:17-35,68-75promises full tests, integration tests, TestPrograms, release artifacts, and merge-blocking regressions on every Tier-1 change. In reality, Windows PR CI runs integration tests and TestPrograms but not the full unit/LSP suite; installer testing is nightly after merge; Linux PR CI does not publish release artifacts; documentation examples are not validated; and MSRV 1.88 is declared but never tested.Fix: document the exact per-platform guarantees that exist today, or add the missing lanes/gates before granting Tier-1 status.
Additional material findings
tests/phase1_correctness_regression_test.rs:46-76usesCARGO_BIN_EXE_wfl, which is Cargo's test-profile binary despite the release-binary claim, and uses unboundedCommand::output(). A looping regression can consume the entire outer job timeout.- Several ignored tests can false-pass: the pattern test discards exit status and accepts
contains("NMATCHES=4"); the Number+Text test accepts any failure whose merged output happens to contain “type.” - #590 is mapped to a focused lexer/parser/type-checker test, not the promised CLI-level end-to-end test.
- #573 is inventoried as open even though #574 implemented binary serving and active byte-equality tests exist.
fuzz_patternmutates only the pattern against one fixed short haystack; ReDoS behavior requires fuzzing crafted pattern/input pairs.- The seeded commands in
fuzz/README.md:51-57pass ignored, absent writable-corpus directories and fail on a fresh clone. The time-boxed example also lacks a per-input-timeout. - Because
fuzz/is excluded and no workflow checks its manifest, API drift can silently break every fuzz target. Add a compile job and commit the standalone fuzz lockfile. - The claim that skipped web programs are covered is overstated: 32 programs carry
CI-SKIP, only three are directly driven byrun_web_tests, and no workflow invokes those scripts. SECURITY.mdnow lists 26.7.x but retains version 26.6.5 in its footer. It also presentsmax_nesting_depthas a recursion defense and says WFL has no cryptographic functions, both inconsistent with current code.- Audit bookkeeping needs correction: the ExecutionBudget file has 32 tests, not 33; the “17 issues” table contains 16 rows; and the #571 test claims slash division while using
divided by. The existing natural-language TestProgram already covers/and can be indexed.
Verification
I inspected the exact head, reproduced the merge conflicts against current main, and checked live GitHub workflow/status data. My review environment does not have a Rust toolchain, so I could not independently compile the branch; the absence of any head-SHA Actions run therefore remains a blocking evidence gap.
GitHub does not allow the PR author to submit a formal
REQUEST_CHANGESreview on their own pull request, so this is posted as a review comment with a request-changes verdict.
Executes the remaining Phase 1 (baseline & containment) tasks of the production-readiness tracker (#610). Production-readiness area: Reliability, Testing, Correctness, Maintenance Tracked issue/risk: #610 Phase 1; defects #569 #571 #592 #578 Gate improved: regression corpus; fuzz targets; supported-platform docs - Regression suite (tests/phase1_correctness_regression_test.rs): the single auditable index of every inventoried correctness defect. Passing guards for fixed defects (#569 action-return-type inference; #571 precedence/division/ modulo/between) and #[ignore]d reproducers for the open ones (#592 bare zero-arg included action; five confirmed #578 bugs) that flip green when Phase 2 fixes land. Also documents that #578's `ends with` misparse no longer reproduces on 26.7.36. - Fuzz targets (fuzz/): standalone cargo-fuzz workspace with fuzz_lexer, fuzz_parser, fuzz_pattern, and fuzz_module_loading, tracked seeds, and a README. Kept out of the stable root build via its own [workspace] and root `exclude = ["fuzz"]`. Type-checks cleanly against the API on stable; the sustained run + corpus retention is Phase 3. - Supported platforms (Docs/reference/supported-platforms.md): three-tier model grounded in the CI matrix (Tier 1 Linux/Windows x86_64; Tier 2 macOS/musl; 32-bit unsupported), toolchain (stable, MSRV 1.88, edition 2024), runtime requirements, and support boundaries. Linked from Docs/README.md and SECURITY.md; refreshed the stale SECURITY.md version-support row to 26.7.x. - Baseline metrics + ExecutionBudget verification + issue inventory/scorecard evidence recorded in three Dev diary entries. Evidence: new regression tests (2 pass, 6 ignored repros verified failing under --ignored); `cargo check --manifest-path fuzz/Cargo.toml` green; `cargo metadata` validates the workspace. Regression protection: every known correctness defect now has an end-to-end test (passing guard if fixed, ignored reproducer if open). Compatibility impact: none (tests, docs, and an excluded fuzz workspace only). Resource impact: none on the shipped runtime. Remaining work: Phase 2 fixes flip the ignored tests green; sustained fuzz run, docs-in-CI, coverage instrumentation, and the consistency suite are tracked. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016egRFdqLHCiAttAGQMoFZw
…base fixups Re-applies the `time::format_description::parse` → `parse_borrowed::<2>` deprecation fix (dropped during the rebase onto main) and addresses the maintainer's request-changes review on #614: Correctness / tests (tests/phase1_correctness_regression_test.rs): - #592 now parameterized across top-level AND action-body (both verified fatal), so a half-fix can't turn it green. - #578: added a verified `format_date` friendly-pattern reproducer; tightened the pattern (exit-status) and Number+Text (non-zero-exit) assertions so they can't false-pass; harness gains a 30s watchdog + drained pipes so a looping regression can't consume the job timeout. - #571 now tests both `divided by` and the `/` symbol as division. - Honestly reframed #578 as an umbrella issue (representative, not exhaustive); noted its nested-`for each` crash did not reproduce and `ends with` is fixed. - Corrected bookkeeping: execution_budget_test has 32 tests (not 33); coverage map header clarified (16 tracked issues + the #610 tracker). Fuzz: - fuzz_module_loading broadened to the real static loading path: checked lex → parse → include/load-module detection → analyze → type-check, with honest scope (no async/FS resolution). - fuzz_pattern now fuzzes pattern/haystack pairs (ReDoS needs both sides). - Committed the standalone fuzz/Cargo.lock; fixed README seed/`-timeout` commands; added a `fuzz-check` CI job so API drift can't silently break the excluded fuzz crate. Docs honesty: - supported-platforms.md: added a per-platform PR-CI coverage table; corrected that the full test suite is Linux-only, the installer test is nightly/ post-merge, MSRV 1.88 is declared-not-tested, docs-in-CI is unmet, and PR CI publishes no artifacts. - SECURITY.md: footer → 26.7.37; corrected "no cryptographic functions" and the `max_nesting_depth`-as-recursion-defense claims; noted ExecutionBudget. - containment diary: replaced the mislabeled "CI-measured on fc21f2f" baseline with a real local `cargo test --all` methodology (DB suites skip when env absent); corrected the run_web_tests coverage claim (no workflow invokes it). Rebased onto current main; kept #613's canonical inventory/scorecard diaries. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016egRFdqLHCiAttAGQMoFZw
6b5bcca to
af91e57
Compare
|
Thanks for the thorough review — addressed in Blocking findings
Additional findings (fixed)False-passing ignored tests (pattern now checks exit status; Number+Text asserts non-zero exit); harness gains a 30s watchdog + drained pipes (no job-timeout blowup); #571 now tests both Two I read differently
Tracked as follow-ups, not done here: a CLI-level #590 guard, exhaustive per-item #578 classification, and the real FS/async module-loading fuzzer (needs a Tokio harness). Generated by Claude Code |
|
|
||
| | Tier | Meaning | What you can rely on | | ||
| |---|---|---| | ||
| | **Tier 1 — Supported** | Built **and** tested on every PR in CI. | A release binary is built and the end-to-end `TestPrograms` + integration-test scripts run on **every** Tier-1 platform; regressions block merges. Coverage is **not identical** across Tier-1 platforms — see *Per-platform PR CI coverage* below for the exact lanes each one runs. | |
| | LSP build + tests | ✅ | ➖ (Linux only) | | ||
| | Clippy `-D warnings` | ✅ | ➖ (Linux only) | | ||
| | Database tests (PostgreSQL + MariaDB) | ✅ | ➖ (Linux only) | | ||
| | Integration-test scripts | ✅ | ✅ | |
| fuzz-check: | ||
| name: Fuzz targets compile | ||
| runs-on: ubuntu-latest | ||
| steps: |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
fuzz/README.md (1)
72-79: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd a language specifier to the fenced code block.
The layout diagram code block lacks a language tag, triggering markdownlint MD040. Use
textorplaintextas the language.✏️ Proposed fix
-``` +fuzz/ + Cargo.toml # standalone cargo-fuzz workspace + fuzz_targets/*.rs # one libFuzzer target per surface + seeds/<target>/ # tracked seed inputs (committed) + corpus/<target>/ # live/evolving corpus (gitignored) + artifacts/<target>/ # crash reproducers (gitignored) +``` +```text fuzz/ Cargo.toml # standalone cargo-fuzz workspace fuzz_targets/*.rs # one libFuzzer target per surface seeds/<target>/ # tracked seed inputs (committed) corpus/<target>/ # live/evolving corpus (gitignored) artifacts/<target>/ # crash reproducers (gitignored)</details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.In
@fuzz/README.mdaround lines 72 - 79, Update the fenced layout diagram in the
README to include a text-compatible language specifier, such as text or
plaintext, while preserving the diagram content unchanged.</details> <!-- cr-comment:v1:75a86650edf5cb35f20fa797 --> _Source: Linters/SAST tools_ </blockquote></details> </blockquote></details> <details> <summary>🤖 Prompt for all review comments with AI agents</summary>Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.Inline comments:
In @.github/workflows/ci.yml:
- Line 36: Update the actions/checkout@v4 step in the fuzz-check job to set
persist-credentials to false, preventing the GITHUB_TOKEN from being stored in
the local Git configuration while preserving the existing checkout behavior.
Nitpick comments:
In@fuzz/README.md:
- Around line 72-79: Update the fenced layout diagram in the README to include a
text-compatible language specifier, such as text or plaintext, while preserving
the diagram content unchanged.</details> <details> <summary>🪄 Autofix (Beta)</summary> Fix all unresolved CodeRabbit comments on this PR: - [ ] <!-- {"checkboxId": "4b0d0e0a-96d7-4f10-b296-3a18ea78f0b9"} --> Push a commit to this branch (recommended) - [ ] <!-- {"checkboxId": "ff5b1114-7d8c-49e6-8ac1-43f82af23a33"} --> Create a new PR with the fixes </details> --- <details> <summary>ℹ️ Review info</summary> <details> <summary>⚙️ Run configuration</summary> **Configuration used**: defaults **Review profile**: CHILL **Plan**: Pro **Run ID**: `b1456b87-9258-489d-aed8-48a5d931dcf7` </details> <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between df56454cace93096fe7985ee7a0160d762667949 and af91e5734a86cad4f76d74c19f004e9d8266eb52. </details> <details> <summary>⛔ Files ignored due to path filters (1)</summary> * `fuzz/Cargo.lock` is excluded by `!**/*.lock` </details> <details> <summary>📒 Files selected for processing (24)</summary> * `.github/workflows/ci.yml` * `Cargo.toml` * `Dev diary/2026-07-13-issue-610-phase-1-containment.md` * `Docs/README.md` * `Docs/reference/supported-platforms.md` * `SECURITY.md` * `fuzz/.gitignore` * `fuzz/Cargo.toml` * `fuzz/README.md` * `fuzz/fuzz_targets/fuzz_lexer.rs` * `fuzz/fuzz_targets/fuzz_module_loading.rs` * `fuzz/fuzz_targets/fuzz_parser.rs` * `fuzz/fuzz_targets/fuzz_pattern.rs` * `fuzz/seeds/fuzz_lexer/seed_check.wfl` * `fuzz/seeds/fuzz_lexer/seed_store.wfl` * `fuzz/seeds/fuzz_module_loading/seed_include.wfl` * `fuzz/seeds/fuzz_module_loading/seed_loadmod.wfl` * `fuzz/seeds/fuzz_parser/seed_action.wfl` * `fuzz/seeds/fuzz_parser/seed_count.wfl` * `fuzz/seeds/fuzz_pattern/seed_digits.txt` * `fuzz/seeds/fuzz_pattern/seed_mixed.txt` * `fuzz/seeds/fuzz_pattern/seed_word.txt` * `src/logging.rs` * `tests/phase1_correctness_regression_test.rs` </details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
- supported-platforms.md: rename the "integration-test scripts" lane to "Rust integration tests (cargo test --test '*')" — CI runs Rust integration tests, not external scripts (Copilot). - ci.yml fuzz-check: add `needs: fmt` (consistency with every other job) and `persist-credentials: false` on checkout (zizmor artipacked; the job only runs `cargo check` and needs no write creds) (Copilot + CodeRabbit). - fuzz/Cargo.toml: edition 2021 → 2024 to match the workspace and surface edition-specific breakages in the fuzz crate (Copilot). Verified it still type-checks under 2024. - fuzz/README.md: add a `text` language tag to the layout fenced block (markdownlint MD040) (CodeRabbit). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016egRFdqLHCiAttAGQMoFZw
logbie
left a comment
There was a problem hiding this comment.
Round 2 — blocker-only review
Three merge blockers remain at f627b4efce.
| //! * #578's *`X ends with Y` misparse* item is also **no longer reproducible** | ||
| //! (fixed alongside #566). | ||
| //! | ||
| //! Full per-item classification of the remaining #578 checkboxes is Phase 2 |
There was a problem hiding this comment.
Blocker — the completed correctness gate still contradicts its own evidence.
This section explicitly says #578 coverage is “representative, not exhaustive” and defers per-item classification, while this file's header says cargo test -- --ignored reproduces every open defect and the containment diary/tracker say every known correctness defect has an end-to-end regression.
#578 still lists known wrong-result/type defects with no mapped test (for example with-form action calls silently concatenating, add to List<Any> dropping the append in test mode, and residual return-type inference failures). Two other mappings also keep the gate inaccurate: #590 points only to an in-process lexer/parser/type-checker test rather than an end-to-end CLI test, and #573 is described as missing binary-read/MIME support even though #574 shipped both with byte-round-trip tests and #573's latest verification recommends closing it.
Either complete and map the per-defect coverage, correct #573, and add the #590 CLI regression, or uncheck/reword the Phase 1 completion claim. The PR cannot serve as exit-gate evidence while it knowingly says both “every” and “representative.”
| use wfl::parser::Parser; | ||
| use wfl::typechecker::TypeChecker; | ||
|
|
||
| fuzz_target!(|data: &[u8]| { |
There was a problem hiding this comment.
Blocker — this still does not fuzz module loading.
The harness parses arbitrary source and runs generic analyzer/type-checker entry points. The two program_has_* calls only inspect an already-parsed AST; no loader path is invoked. It therefore cannot reach path resolution/canonicalization, bounded file reads, circular/import-depth enforcement, parent-scope construction, or module execution—the loader-specific behavior in Interpreter's LoadModuleStatement and IncludeStatement paths.
Calling this the “static half” documents the gap but does not satisfy #610's checked Phase 1 task to establish a module-loading fuzz target or the diary's claim that all four required surfaces are covered. Drive a real temporary-file load/include through an interpreter/Tokio harness, or rename this as frontend/static-analysis fuzzing and leave the module-loading item open.
| while compiling the ~70 release test binaries (`rustc-LLVM ERROR: No space left | ||
| on device`) — an environmental limit, **not** a test failure — so the | ||
| authoritative full-suite aggregate is deferred to **CI on the pushed head SHA** | ||
| (the `clippy-and-test` job runs the full `cargo test` on `ubuntu-latest`). Until |
There was a problem hiding this comment.
Blocker — current CI cannot provide the “authoritative full-suite aggregate” promised here.
The head-SHA CI run is green, but its cargo test command tests only the root package: the log totals 1206 passed / 0 failed / 24 ignored across 76 result suites. The separate LSP command contributes 69 tests, while the wflpkg package's 204 tests are never run. Consequently the documented ≈1479/95 aggregate is still derived, and this workflow cannot confirm it as a full-workspace baseline.
Because “record baseline test count” is marked complete and this paragraph explicitly defers authority to CI, add cargo test --all or an explicit cargo test -p wflpkg lane, then record the observed, scope-labeled head-SHA counts with the workflow link.
…ace) Blocker 1 — resolve the "every vs representative" correctness-gate contradiction: - Reword the completion claim to per-issue coverage with an explicit, representative (not exhaustive) #578 sample; scope the "reproduces every open defect" line to defects encoded in this file. - Add a CLI-level end-to-end #590 guard (complements the in-process test the review noted), and a `with`-form-concat #578 reproducer (still reproduces). - Correct #573 to FIXED: binary read/write + MIME shipped in #574 with byte round-trip tests; the issue's own latest verification recommends closing. - Re-verified with the release binary: #578's `add`-to-List<Any> test-mode drop and `double of 5 minus 1` inference items no longer reproduce (fixed), so they are not encoded — documented as such. Blocker 2 — fuzz_module_loading did not fuzz module loading: - Rename it to `fuzz_frontend` (it fuzzes the static frontend: checked lex → parse → analyze → type-check). Mark the module-loading fuzz surface as an explicitly OPEN Phase 1 item (safe async loader harness is non-trivial — executing fuzzer WFL would also spawn subprocesses/network/web/file writes). Diary/exit-gate now say three-of-four surfaces covered, not four. Blocker 3 — CI could not provide the "authoritative full-suite aggregate": - ci.yml "Run Tests" now runs `cargo test --workspace` (was root-package-only, which skipped wflpkg's 204 tests). Record the observed scope-labeled head-SHA numbers (root 1206/0/24 across 76 suites, wfl-lsp 69, wflpkg 204) and note the authoritative combined aggregate now comes from the --workspace CI run. Verified: suite 3 passed / 9 ignored; all 9 ignored reproduce under --ignored; fmt + clippy -D warnings clean; fuzz crate type-checks after the rename. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016egRFdqLHCiAttAGQMoFZw
|
Round-2 blockers addressed in Blocker 1 — "every vs representative" contradiction.
Blocker 2 — fuzz_module_loading didn't fuzz module loading.
Blocker 3 — CI wasn't a full-workspace aggregate.
Verification: suite is 3 passed / 9 ignored; all 9 ignored reproduce under Net: Phase 1 is not marked fully complete. Open items carried forward and tracked: the module-loading fuzz target, recording the Generated by Claude Code |
| display (p_unary of 3)[\"val\"]\n", | ||
| ); | ||
| assert!( | ||
| !out.contains("Cannot index into Nothing"), | ||
| "self-recursive indexed result must not be typed Nothing (#590): {out}" | ||
| ); | ||
| assert!( | ||
| !out.contains("found Nothing"), | ||
| "no spurious Nothing diagnostic on the recursive result (#590): {out}" | ||
| ); | ||
| assert_eq!(code, Some(0), "program should exit 0 (#590): {out}"); | ||
| // The base case returns `other of 0` → map {"val": 0}; indexing "val" prints 0. | ||
| assert!( | ||
| out.contains('0'), | ||
| "program must run and print its value (#590): {out}" | ||
| ); |
| "include from \"mod.wfl\"\n\ | ||
| define action called run_it:\n store x as greet\n return x\nend action\n\ | ||
| display run_it\n", |
|
|
||
| | Tier | Meaning | What you can rely on | | ||
| |---|---|---| | ||
| | **Tier 1 — Supported** | Built **and** tested on every PR in CI. | A release binary is built and the end-to-end `TestPrograms` + Rust integration tests (`cargo test --test '*'`) run on **every** Tier-1 platform; regressions block merges. Coverage is **not identical** across Tier-1 platforms — see *Per-platform PR CI coverage* below for the exact lanes each one runs. | |
| | LSP build + tests | ✅ | ➖ (Linux only) | | ||
| | Clippy `-D warnings` | ✅ | ➖ (Linux only) | | ||
| | Database tests (PostgreSQL + MariaDB) | ✅ | ➖ (Linux only) | | ||
| | Rust integration tests (`cargo test --test '*'`) | ✅ | ✅ | |
| | `fuzz_pattern` | Pattern grammar + compiler + VM (ReDoS surface) | `pattern\0haystack` pair → `create pattern` parse → `CompiledPattern::compile` → `find_all(haystack)` | | ||
| | `fuzz_frontend` | Compiler **frontend** on arbitrary source: checked lex → parse → include/load-module detection → analyze → type check | `lex_wfl_with_positions_checked` → `Parser::parse` → `program_has_includes`/`program_has_load_module` → `Analyzer::analyze` → `TypeChecker::check_types` | | ||
|
|
- #590 CLI guard: assert the exact labeled marker `VAL=0` (was `contains('0')`, which could match unrelated output) — program now prints `display "VAL=" with …`. - #592 action-body reproducer: invoke run_it with an explicit `call` (was a bare `display run_it`) so the test stays focused on included-action name resolution and doesn't depend on top-level bare-call semantics. Still reproduces (exit 3). - fuzz/README: add a naming note that the frontend target was renamed from `fuzz_module_loading` → `fuzz_frontend` (for readers cross-referencing older PR text). Declined (incorrect): Copilot flagged `cargo test --test '*'` in the platform doc as invalid — it is a valid cargo glob and the exact command `ci.yml` runs (line 165), so the doc accurately mirrors CI; left unchanged. Verified: suite 3 passed / 9 ignored; all 9 ignored reproduce; fmt + clippy clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016egRFdqLHCiAttAGQMoFZw
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 24 out of 25 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
.github/workflows/ci.yml:105
cargo test --workspacealready runs thewfl-lspcrate's tests (since it is a workspace member), so the subsequent explicitBuild LSP/Run LSP Testssteps duplicate compilation and test execution, increasing CI time without adding coverage. Removing the redundant steps keeps the job faster while preserving the same gating signal.
# Build LSP to catch Send/Sync regressions
- name: Build LSP
run: cargo build -p wfl-lsp --verbose
# Run LSP tests
| |---|---|---|---| | ||
| | **Linux (glibc)** | `x86_64` | **Tier 1** | `ci.yml` builds + tests on `ubuntu-latest`: unit/integration tests, Clippy (`-D warnings`), database tests (PostgreSQL + MariaDB), and the `TestPrograms` runner. | | ||
| | **Windows** | `x86_64` (`x86_64-pc-windows-msvc`) | **Tier 1** | `ci.yml` runs the integration + `TestPrograms` matrix on `windows-latest`. The MSI installer (`cargo-wix`) and its smoke test run in `nightly.yml` **after** merge, not on PRs. | | ||
| | **macOS** | `x86_64`, `aarch64` (Apple Silicon) | **Tier 2** | Builds from source (`installation.md` documents the flow) but is **not** in CI. Supported best-effort until a macOS CI lane is added. | |
…t step - supported-platforms.md: make the macOS row's `installation.md` reference a proper relative link (`../02-getting-started/installation.md`). - ci.yml: remove the now-redundant `Run LSP Tests` step — `cargo test --workspace` (added earlier in this PR) already runs wfl-lsp's tests, so the separate `cargo test -p wfl-lsp` only duplicated them. Kept the explicit `Build LSP` step as a focused Send/Sync build gate. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016egRFdqLHCiAttAGQMoFZw
| # Run Clippy for code quality | ||
| - name: Run Clippy | ||
| run: cargo clippy --all-targets -- -D warnings |
There was a problem hiding this comment.
Valid gap, but deferring it out of this containment PR. I verified locally that flipping the CI step to cargo clippy --workspace --all-targets -- -D warnings is not currently green — it surfaces ~10 pre-existing lint findings across wfl-lsp/wflpkg test code that root-package clippy never saw: unused imports (lsp_end_to_end_validation_test.rs, lsp_performance_stability_test.rs), 3× unused document_text (lsp_completion_test.rs), an unread warning_count field, a map_or(false, …), a single-char push_str, and a field_reassign_with_default in wflpkg. Clearing those is a worthwhile but separate lint-cleanup that touches two crates this Phase-1 containment PR otherwise doesn't, so I'd rather not balloon it here — tracking it as a follow-up. The related, actually-blocking CI gap on this workflow — fuzz-check missing from bump-version.needs — is fixed in this PR (4bcd6d5).
Generated by Claude Code
| # The fuzz crate is a standalone workspace excluded from the root build, so a | ||
| # normal `cargo build` never compiles it — API drift in wfl could silently | ||
| # break every fuzz target. This job type-checks the targets against the current | ||
| # API on stable (libFuzzer/nightly is only needed to actually *run* them). | ||
| fuzz-check: | ||
| name: Fuzz targets compile | ||
| runs-on: ubuntu-latest | ||
| needs: fmt |
| // Require an explicit non-zero exit. NOT `code != Some(0)`, which would also | ||
| // accept a timeout kill (`code == None`) and let a future hang pass as green. | ||
| assert!( | ||
| matches!(code, Some(c) if c != 0), | ||
| "`Number plus Text` should be rejected with a non-zero exit — not exit 0 and not a hang/timeout (#578): {out}" | ||
| ); | ||
| } |
| fuzz-check: | ||
| name: Fuzz targets compile | ||
| runs-on: ubuntu-latest | ||
| needs: fmt |
There was a problem hiding this comment.
Blocker: fuzz-check is the only compile gate for the excluded fuzz workspace, but the write-capable bump-version job still has needs: [fmt, clippy-and-test, integration-tests, database-tests, run-wfl-programs] and omits this job. On a push to main, fuzz-check can fail while bump-version still commits, pushes, and tags a new version—contradicting the workflow’s “only after ALL checks pass” guarantee. The generated bump commit includes [skip ci], so that published version does not get a corrective rerun either. Add fuzz-check to bump-version.needs so a broken fuzz workspace cannot be versioned/tagged.
…t (PR #614) - ci.yml: add `fuzz-check` to `bump-version.needs`. It was the only compile gate for the excluded fuzz workspace, but the write-capable bump-version job (documented "only after ALL checks pass") omitted it, so a push to main could tag a version while fuzz-check was red — and the bump commit carries [skip ci], so no corrective rerun follows. (Maintainer blocker.) - phase1 regression test: the #578 Number+Text reproducer required a non-zero exit to count as "rejected", but WFL type errors are non-fatal (a "Type checking warnings:" diagnostic, exit 0) — only ExecutionBudget breaches are fatal (src/main.rs). That baked in an exit-code policy the runtime doesn't use for type errors, so the reproducer could never flip green on a natural fix. Accept a non-zero exit OR an explicit type-checker diagnostic on a completed run; a timeout (code == None) still fails both branches. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016egRFdqLHCiAttAGQMoFZw
| let dir = TempDir::new().expect("tempdir"); | ||
| for (name, content) in files { | ||
| let path = dir.path().join(name); | ||
| if let Some(parent) = path.parent() { | ||
| fs::create_dir_all(parent).expect("mkdir"); | ||
| } | ||
| fs::write(&path, content).expect("write file"); | ||
| } | ||
| let entry_path = dir.path().join(entry); | ||
| let mut child = Command::new(wfl_exe()) | ||
| .arg(&entry_path) | ||
| .current_dir(dir.path()) | ||
| .stdout(Stdio::piped()) | ||
| .stderr(Stdio::piped()) | ||
| .spawn() | ||
| .expect("failed to spawn WFL"); |
… (PR #614) `run_files` spawned the wfl binary inheriting the ambient environment, so the child could read a machine-global `/etc/wfl/wfl.cfg` (or the legacy `/etc/wfl/.wflcfg` fallback) and silently change timeouts/limits/behavior, making the suite non-hermetic and flaky outside CI. Pin `WFL_GLOBAL_CONFIG_PATH` to an empty `NamedTempFile` for the child. The file must exist and be empty: per src/config.rs the loader falls back to the legacy `/etc/wfl/.wflcfg` when the configured path is missing, so a nonexistent path would not isolate it. The temp file lives outside the working dir (directory-listing reproducers can't see it) and stays in scope until after the child exits. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016egRFdqLHCiAttAGQMoFZw
logbie
left a comment
There was a problem hiding this comment.
One merge blocker remains; details inline.
|
|
||
| [[package]] | ||
| name = "wfl" | ||
| version = "26.7.37" |
There was a problem hiding this comment.
Blocker: this committed lock is made stale by the automatic post-merge version bump. fuzz/Cargo.toml is a separate workspace that path-depends on root wfl, so this entry records the root package version. Every successful main push runs scripts/bump_version.py --update-all, but update_cargo_lock() hard-codes only the root Cargo.lock, and MODIFIED_FILES never stages fuzz/Cargo.lock. The bot will therefore commit/tag root 26.7.38 with [skip ci] while leaving this at 26.7.37; the next cargo check --locked --manifest-path fuzz/Cargo.toml must fail because the lock needs updating. Adding fuzz-check to the pre-bump needs list does not validate the mutation. Please regenerate and stage fuzz/Cargo.lock in the bump path (including retry bumps), and run the locked fuzz check after the bump before pushing/tagging.
logbie
left a comment
There was a problem hiding this comment.
One additional audit-evidence blocker surfaced in the final source-level reachability check; details inline.
| | #573 | Web server cannot serve binary content (fonts, images) | **Medium** | Real limitation (file read + HTTP body are text/UTF-8 only); blocks self-hosting static assets. Not a regression in existing behavior. | | ||
| | #600 | Native TLS: SNI / multiple certificates on one `:443` | **Post-production-readiness** | Single-cert HTTPS works; multi-cert/SNI is a multi-tenant deployment enhancement, not a release-gate blocker. | | ||
| | ~~#573~~ | Web server cannot serve binary content (fonts, images) | **Fixed (correction)** | **Reclassified: this was recorded open in error.** PR #574 shipped binary read (`read binary from …`), binary write, lossless byte round-trip, and MIME helpers *before* this inventory, guarded by `web_server_binary_test.rs`, `binary_io_test.rs`, and `binary_file_and_mime_test.wfl`. The issue's own latest verification (2026-07-06) recommends closing; it is open on GitHub only pending a close click. | | ||
| | #600 | Native TLS refactor (SNI / multi-cert) — **also the vehicle for the `rustls-webpki` security bump** | **High (security)** | **Reclassified from "post-production."** #600 carries open **high-severity Dependabot alert #49** (`rustls-webpki` DoS via panic), still live in `Cargo.lock` through `warp 0.3.7 → tokio-rustls 0.25.0 → rustls 0.22.4 → rustls-webpki 0.102.8`. `warp 0.3.7` pins that TLS stack, so **no in-line bump exists** — the fix rides #600's TLS rewrite. It therefore blocks the mandatory *no-open-high-severity-security* release gate; the SNI/multi-cert ergonomics are secondary. (This PR classifies the risk; it does **not** implement the rewrite.) See [#600 update](https://github.com/WebFirstLanguage/wfl/issues/600#issuecomment-4956256654) and [#610 analysis](https://github.com/WebFirstLanguage/wfl/issues/610#issuecomment-4956252322). | |
There was a problem hiding this comment.
Blocker: this reclassification treats vulnerable dependency presence as WFL exploitability. GHSA-82j2-j2ch-gfr8 explicitly says the panic requires both opt-in RevocationOptions and attacker-influenced CRL bytes, and that default rustls configurations are unaffected. WFL only configures warp with .tls().cert_path(...).key_path(...); warp 0.3.7 defaults client_auth to TlsClientAuth::Off and builds that path with with_no_client_auth(), with no CRL/RevocationOptions input. The literal “no open High finding” policy gate can remain administratively open until alert #49 is triaged, but this audit cannot classify #600 itself as a reachable High WFL security defect or assert that its TLS rewrite is the required remediation on the evidence here. Please record/dismiss the alert as “vulnerable code not used,” or provide a concrete reachable CRL path; keep #600’s SNI priority separate.
`fuzz/` is a separate workspace that path-depends on root `wfl`, so `fuzz/Cargo.lock` pins the root version too. `scripts/bump_version.py`'s `update_cargo_lock()` only refreshed the root lock and never staged `fuzz/Cargo.lock`, so every post-merge `--update-all` bump left the fuzz lock stale at the old version. Because the bump commit carries `[skip ci]`, the breakage surfaced only on the *next* PR, whose `cargo check --locked --manifest-path fuzz/Cargo.toml` (`fuzz-check`) would fail. Adding `fuzz-check` to `bump-version.needs` gates the pre-bump state but not the mutation the bump itself makes. Add `update_fuzz_cargo_lock()`: after the root lock is updated it runs `cargo update -p wfl --manifest-path fuzz/Cargo.toml`, verifies the fuzz lock now records the new version, runs the same `cargo check --locked` gate to prove the mutation is consistent before anything is committed/tagged, and stages `fuzz/Cargo.lock`. It's called from the `--update-all` path, so retry bumps (which re-run the whole script) are covered. Verified locally: the sync command re-pins a deliberately stale fuzz lock back to the root version. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016egRFdqLHCiAttAGQMoFZw
A source-level reachability re-review shows the earlier "#600 = High (security)" reclassification overreached: it treated the *presence* of `rustls-webpki` (Dependabot alert #49 / GHSA-82j2-j2ch-gfr8) in the dependency graph as WFL exploitability. The advisory's panic requires opt-in `RevocationOptions` AND attacker-controlled CRL bytes; default rustls configs are unaffected. WFL's only TLS setup is `warp::serve(routes).tls().cert_path().key_path()` (src/interpreter/mod.rs:6441) with client auth off and NO CRL / `RevocationOptions` anywhere (verified by grep), so the vulnerable path is not reachable. Re-disposition across the audit evidence: alert #49 = "vulnerable code not used"; #600 is the separate SNI / multi-cert enhancement (post-production), NOT a reachable High WFL defect, and its TLS rewrite is not established as required remediation. The literal no-open-high-severity-security policy gate may stay administratively open until #49 is formally triaged. Updated the inventory #600 row + severity legend (High now requires a *reachable* advisory, not mere presence) + exit-gate read, the containment exit-gate read, and the regression-index header row. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016egRFdqLHCiAttAGQMoFZw
| let stdout = out_thread.join().unwrap_or_default(); | ||
| let stderr = err_thread.join().unwrap_or_default(); | ||
| drop(dir); | ||
| let combined = format!("{stdout}{stderr}"); |
) `run_files` joined the captured stdout and stderr with no delimiter, so a `contains(...)` / exact-line assertion could false-match a substring that straddled the boundary (end of stdout + start of stderr). Insert a newline separator between the two captures. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016egRFdqLHCiAttAGQMoFZw
There was a problem hiding this comment.
🧹 Nitpick comments (1)
scripts/bump_version.py (1)
255-273: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate
wflversion-extraction regex.The
[[package]] name = "wfl" version = "..."regex and comparison logic (lines 259-273) is a verbatim copy of the same block inupdate_cargo_lock(lines 190-204). Consider extracting a shared helper, e.g._extract_wfl_lock_version(lock_path), used by both functions to avoid the regex drifting out of sync if theCargo.lockformat ever changes.♻️ Suggested extraction
+def _extract_wfl_lock_version(lock_path): + """Extract the pinned `wfl` package version from a Cargo.lock file.""" + with open(lock_path, "r") as f: + content = f.read() + match = re.search( + r'\[\[package\]\]\s*name = "wfl"\s*version = "([^"]+)"', + content, + re.DOTALL, + ) + if not match: + print(f"Error: Could not find WFL package version in {lock_path}") + sys.exit(1) + return match.group(1)Then both
update_cargo_lockandupdate_fuzz_cargo_lockcall_extract_wfl_lock_version(...)instead of duplicating the regex.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/bump_version.py` around lines 255 - 273, Extract the duplicated WFL lock-version parsing and validation setup into a shared helper such as _extract_wfl_lock_version(lock_path), reusing the existing regex and file-reading behavior. Update both update_cargo_lock and update_fuzz_cargo_lock to call the helper and retain their existing mismatch handling and error outcomes.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@scripts/bump_version.py`:
- Around line 255-273: Extract the duplicated WFL lock-version parsing and
validation setup into a shared helper such as
_extract_wfl_lock_version(lock_path), reusing the existing regex and
file-reading behavior. Update both update_cargo_lock and update_fuzz_cargo_lock
to call the helper and retain their existing mismatch handling and error
outcomes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e49a1a76-0c56-41eb-94ba-e81113a6ae78
📒 Files selected for processing (5)
.github/workflows/ci.ymlDev diary/2026-07-13-issue-610-phase-1-containment.mdDev diary/2026-07-13-issue-610-phase-1-inventory.mdscripts/bump_version.pytests/phase1_correctness_regression_test.rs
🚧 Files skipped from review as they are similar to previous changes (2)
- .github/workflows/ci.yml
- tests/phase1_correctness_regression_test.rs
Extract `_extract_wfl_lock_version(lock_path)` and use it from both `update_cargo_lock` (root) and `update_fuzz_cargo_lock` (fuzz workspace), removing the duplicated `[[package]] name = "wfl"` parse so the two copies can't drift if the Cargo.lock format ever changes. Behaviour is unchanged: same regex, same "not found" hard-fail. (CodeRabbit nit.) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016egRFdqLHCiAttAGQMoFZw
logbie
left a comment
There was a problem hiding this comment.
One merge blocker remains; it is tracker-only and requires no code expansion. Details inline.
| | #573 | Web server cannot serve binary content (fonts, images) | **Medium** | Real limitation (file read + HTTP body are text/UTF-8 only); blocks self-hosting static assets. Not a regression in existing behavior. | | ||
| | #600 | Native TLS: SNI / multiple certificates on one `:443` | **Post-production-readiness** | Single-cert HTTPS works; multi-cert/SNI is a multi-tenant deployment enhancement, not a release-gate blocker. | | ||
| | ~~#573~~ | Web server cannot serve binary content (fonts, images) | **Fixed (correction)** | **Reclassified: this was recorded open in error.** PR #574 shipped binary read (`read binary from …`), binary write, lossless byte round-trip, and MIME helpers *before* this inventory, guarded by `web_server_binary_test.rs`, `binary_io_test.rs`, and `binary_file_and_mime_test.wfl`. The issue's own latest verification (2026-07-06) recommends closing; it is open on GitHub only pending a close click. | | ||
| | #600 | Native TLS ergonomics: SNI / multiple certificates on one `:443` | **Post-production-readiness** (SNI) · Dependabot alert #49 = *vulnerable code not used* | **Correction (source-level re-review).** An earlier revision reclassified this **High (security)**, treating the *presence* of `rustls-webpki` (alert #49, [GHSA-82j2-j2ch-gfr8](https://github.com/advisories/GHSA-82j2-j2ch-gfr8)) in the dependency graph as WFL exploitability. That overreached. The advisory's panic requires opt-in `RevocationOptions` **and** attacker-controlled CRL bytes; default rustls configs are unaffected. WFL's only TLS setup is `warp::serve(routes).tls().cert_path(…).key_path(…)` (`src/interpreter/mod.rs:6441`); warp 0.3.7 defaults client auth to `TlsClientAuth::Off` / `with_no_client_auth()`, and WFL configures **no** CRL / `RevocationOptions` anywhere (verified by grep) — so the vulnerable path is **not reachable**. Disposition: record/dismiss alert #49 as *"vulnerable code not used."* #600 itself is the **SNI / multi-cert enhancement** (post-production); it is **not** a reachable High WFL security defect, and its TLS rewrite is not established as required remediation on this evidence. The literal *no-open-High-severity-security* policy gate may remain administratively open until #49 is formally triaged. SNI priority is tracked on #600 independently. | |
There was a problem hiding this comment.
Blocker: this corrected row still conflicts with the tracked issue’s own current record. #600’s latest update still says #600 is the High alert’s fix vehicle, calls the dependency chain live, and raises the issue from post-production priority; #600 has no superseding correction. The correction on #610 does not repair the canonical #600 thread. Because this PR’s Phase-1 deliverable is an auditable open-issue inventory, leaving the source issue with the opposite classification makes the evidence incoherent. Please post or edit a superseding #600 update with the verified RevocationOptions/CRL reachability disposition and keep the SNI priority separate. This is tracker-only; no additional code commit is needed.
Executes most of the remaining Phase 1 (baseline & containment) tasks of the production-readiness tracker (#610). Phase 1 is not fully complete — three Phase 1 items remain open (matching the three unchecked Phase 1 boxes on #610) and are called out under Remaining work below.
Production-readiness area: Reliability, Testing, Correctness, Maintenance, Security
Tracked issue/risk: #610 Phase 1; defects #569 #571 #573 #578 #590 #592; #600 (SNI / multi-cert enhancement — post-production); Dependabot alert #49 (
rustls-webpki) present but not reachable → "vulnerable code not used"Gate improved: regression corpus; fuzz targets (3 of 4 surfaces); supported-platform docs; full-workspace CI
Regression suite (
tests/phase1_correctness_regression_test.rs): passing binary-level guards for fixed defects (Type checker infers every user-defined action's return type asNothing, causing spurious "Expected Text but found Nothing" at call sites #569 action-return-type inference; Docs audit: natural-language constructs that read like valid WFL but don't work (precedence,/,finally,between, error binding) #571 precedence + bothdivided byand the/symbol as division + modulo + between; Type checker: self-recursive action's result is typedNothinginside its own body (false "Cannot index into Nothing") #590 self-recursive indexed result, CLI-level) and#[ignore]d reproducers for the open ones (Zero-argument include-exposed action referenced by bare name is fatal at top level & in action bodies (Variable '<name>' is not defined) — the call form #580's fix didn't cover #592 bare zero-arg included action at top level and inside an action body; six confirmed Follow-up to #571: remaining language/stdlib rough edges (inference, repeat N times, text→number, pattern-VM, filesystem glob, and more) #578 bugs) that flip green when Phase 2 fixes land. The harness is hermetic (pinsWFL_GLOBAL_CONFIG_PATHso a machine-global config can't perturb it). This does NOT complete the "convert every known correctness defect" task — Follow-up to #571: remaining language/stdlib rough edges (inference, repeat N times, text→number, pattern-VM, filesystem glob, and more) #578 is an umbrella and only its reproducible confirmed bugs are encoded; exhaustive per-item Follow-up to #571: remaining language/stdlib rough edges (inference, repeat N times, text→number, pattern-VM, filesystem glob, and more) #578 classification (a Phase 1 task) remains open (fixing those defects is Phase 2). Corrections from re-verification: Web server cannot serve binary content (fonts, images) — response body and file reads are text-only #573 is fixed (binary serving + MIME shipped in Add binary file I/O and MIME type support for web assets #574), and Follow-up to #571: remaining language/stdlib rough edges (inference, repeat N times, text→number, pattern-VM, filesystem glob, and more) #578'sends withmisparse /add-to-List/double of 5 minus 1items no longer reproduce.Fuzz targets (
fuzz/): standalone cargo-fuzz workspace withfuzz_lexer,fuzz_parser,fuzz_pattern, andfuzz_frontend(the compiler frontend — formerly the misleadingly-namedfuzz_module_loading), tracked seeds, a committedCargo.lock, and a README. Afuzz-checkCI job type-checks them on stable with--locked,bump-versionnowneedsit, andscripts/bump_version.pykeepsfuzz/Cargo.lockin sync during the auto-bump so the[skip ci]bump can't silently break the locked check. Module-loading fuzzing is NOT done — three of the four required surfaces are covered; the real FS/async loader needs a sandboxed harness and is carried forward.Supported platforms (
Docs/reference/supported-platforms.md): three-tier model grounded in the actual CI matrix, with a per-platform PR-CI coverage table (full unit/LSP/clippy/DB suite is Linux-only; installer test is nightly/post-merge; MSRV 1.88 declared-not-gate-tested; docs-in-CI unmet; no PR artifacts). Linked fromDocs/README.mdandSECURITY.md(whose footer version, "no cryptographic functions", andmax_nesting_depth-as-recursion claims were corrected).CI: the "Run Tests" step now runs
cargo test --workspace(was root-package-only, which skippedwflpkg's tests). Baseline metrics + ExecutionBudget verification + inventory/scorecard evidence are recorded in the Dev diary entries.Native TLS web server: support SNI / multiple certificates on a single :443 listener #600 / Dependabot alert Fix Linux smoke test to find DEB file in correct location #49 — vulnerable dependency present, code path NOT reachable: a source-level re-review corrected an earlier over-classification. Alert Fix Linux smoke test to find DEB file in correct location #49 (
rustls-webpki, GHSA-82j2-j2ch-gfr8) is in the dependency graph, but its panic needs opt-inRevocationOptionsand attacker-controlled CRL bytes; WFL's only TLS setup iswarp::serve(routes).tls().cert_path(…).key_path(…)(src/interpreter/mod.rs:6441) with client auth off and no CRL /RevocationOptionsconfigured anywhere, so the vulnerable path is unreachable. Disposition: "vulnerable code not used." Native TLS web server: support SNI / multiple certificates on a single :443 listener #600 itself is the separate SNI / multi-cert enhancement (post-production) — not a reachable High WFL security defect, and its TLS rewrite is not established as required remediation. The literal no-open-high-severity-security policy gate may remain administratively open until Fix Linux smoke test to find DEB file in correct location #49 is formally triaged.Evidence: regression suite 3 passed / 9 ignored (every ignored reproducer verified failing under
--ignored); measured full-workspace CI run 29240959575: 1480 passed / 0 failed / 25 ignored across 95 result suites;cargo check --locked --manifest-path fuzz/Cargo.tomlgreen.Regression protection: each fixed defect has a passing guard; each open defect has an
#[ignore]d reproducer that flips green when fixed.Compatibility impact: none (tests, docs, CI, and an excluded fuzz workspace; plus one deprecation fix in
src/logging.rs).Resource impact: none on the shipped runtime.
Remaining work — the three open Phase 1 items not closed by this PR: (1) a module-loading fuzz target (safe async harness); (2) exhaustive per-item #578 classification (Phase 1; fixing #578 is Phase 2); (3) a line-coverage baseline (no coverage tool is instrumented yet, so the "record baseline coverage" task is only partially done). Phase 2/3 hand-offs (separate, not Phase 1): the consistency suite, docs-examples-in-CI, the sustained fuzz run + corpus retention, and per-limit adversarial tests. (Separately, Dependabot alert #49 /
rustls-webpkiis present but not reachable in WFL — dispositioned "vulnerable code not used" above; #600's SNI enhancement is post-production.)Co-Authored-By: Claude Opus 4.8
Claude-Session: https://claude.ai/code/session_016egRFdqLHCiAttAGQMoFZw
Open in Devin Review
Summary by CodeRabbit
New Features
Bug Fixes
Chores
Summary by CodeRabbit
New Features
Documentation
Bug Fixes